Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PropertyInfo][Serializer] Add limited generics support to PhpStanExtractor #47556

Open
wants to merge 1 commit into
base: 7.2
Choose a base branch
from

Conversation

tigitz
Copy link
Contributor

@tigitz tigitz commented Sep 12, 2022

Q A
Branch? 6.2
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #45071
License MIT
Doc PR

This PR aims to provides a limited support of generics serialization with the help of PhpStanExtractor.

Basically achieving:

class Foo {
    /**
     * @param Generic<\stdClass> $bar
     */
    public function __construct(public $bar){}
}

/**
 * @template T
 */
class Generic {
    /**
     * @param T $variableType
     */
    public function __construct(public $variableType){}
}

$dummy = new \stdClass();
$dummy->prop = 'foo';

$normalizers->normalize(new Foo(bar:new Generic($dummy)));
// ['bar' => ['variableType' => ['prop' => 'foo']]]

$normalizers->denormalize(['bar' => ['variableType' => ['prop' => 'foo']]]);
// new Foo(bar:new Generic($dummy))

It introduces an internal normalization_outer_class_property key in the normalization context to provide the property extractor the type found in the generic.

Its keeping BC with iterables types documented as generics (e.g. array<>, Collection<>)

It's an early stage feature as generics can be very flexible and I encourage everyone to please test it on their own projects and help me catch all the remaining use cases.

It's limited in the sense that it doesn't support:

  • Nested generics
  • Doesn't support @implements Collection<T>

Debatable topics:

  • normalization_outer_class_property key naming
  • Should it requires an opt-in / opt-out mechanism ? As it stands, for each OuterObject->InnerObject normalizations there's an added key/value in the context. Hopefully it won't be a huge performance impact but maybe we could let users opting-out.
  • I've refactored a bit the handling of promoted property type extraction so that, if both present, @var type will take precedence over @param type.

To Do

  • Update the ContextBuilder ?

@carsonbot carsonbot added this to the 6.2 milestone Sep 12, 2022
@carsonbot carsonbot changed the title [Serializer][PropertyInfo] Add limited generics support to PhpStanExtractor [PropertyInfo][Serializer] Add limited generics support to PhpStanExtractor Sep 12, 2022
@tigitz
Copy link
Contributor Author

tigitz commented Sep 12, 2022

@nicolas-grekas Thanks again for giving me the push I needed to complete and polish my POC for this PR. OSS dynamics FTW as you say :)

It now seems much less insurmountable to contribute 🚀.

@carsonbot
Copy link

Hey!

I think @Korbeil has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@norkunas
Copy link
Contributor

Could someone review this? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PropertyInfo] Add support for generics
5 participants